Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Handle DOM storage being disabled (#197798) #198358

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

kibanamachine
Copy link
Contributor

Backport

This will backport the following commits from main to 8.x:

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

Questions ?

Please refer to the Backport tool documentation

## Summary

This PR aims to improve the message shown to users when Kibana can't be
started due to disabled DOM storage (elastic#121189).
The visuals here follow the same pattern as other fatal errors (see
elastic#186609)

![image](https://github.com/user-attachments/assets/19832830-49e3-4789-9b83-0c1f14d7980d)

The `isDomStorageDisabled` check has to be done before `CoreService`
gets instantiated because of issues described below.

Closes: elastic#121189

## The issue

What actually happens when you disable all cookies in a browser? Aside
from cookies, the browser disables the whole DOM storage -
`localStorage` and `sessionStorage`. Trying to access those will result
in an error.

`getSessionId`https://github.com/elastic/kibana/blob/3bc5e2db73799dc9c7831b6f9da4a52063cf112f/packages/core/analytics/core-analytics-browser-internal/src/get_session_id.ts#L17
and
`isSidenavCollapsed$`https://github.com/elastic/kibana/blob/3bc5e2db73799dc9c7831b6f9da4a52063cf112f/packages/core/chrome/core-chrome-browser-internal/src/chrome_service.tsx#L91

Both of those try to access either `localStorage` or `sessionStorage`
and both of those are triggered when you create an instance of
`CoreSystem` which gets instantiated in `kbn_bootstrap`
https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/root/core-root-browser-internal/src/kbn_bootstrap.ts#L42

Trying to access DOM storage in `CoreSystem` will cause it to throw an
error and this means that
`FatalErrorService`https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/fatal-errors/core-fatal-errors-browser-internal/src/fatal_errors_service.tsx#L32
will never instantiate and the
`failure`https://github.com/elastic/kibana/blob/6ef03697460aba0d3774c0c03fb7fb58c76c00bd/packages/core/rendering/core-rendering-server-internal/src/bootstrap/render_template.ts#L68
function which styles the errors and makes them visible will never
trigger and all the user will see is permament `Loading Kibana` spinner.

Wrapping `getSessionId` and `isSidenavCollapsed$` in `try-catch` block
allows `FatalErrorService` to work properly, which will catch an
unhandled exception (`Detected an unhandled Promise rejection.`) with an
error about `sessionStorage` being disabled, which gets thrown by
`LicensingPlugin` (and possibly in other places). This is not an actual
solution though - this behavior would happen again if another line of
code trying to access DOM storage gets added to `CoreSystem`.

I think it would be best to handle this directly in `kbn_bootstrap.ts`
by some check like the one below:
```javascript
const isDOMStorageDisabled = () => {
    try {
      const key = 'kbn_bootrasrap_domStorageEnabled';
      sessionStorage.setItem(key, 'true');
      sessionStorage.removeItem(key);
      return false;
    } catch (e) {
      return true;
    }
  };
const domStorageDisabled = isDOMStorageDisabled()
/*
  ...some additonal logic
*/
```
This would then require some error displaying logic that doesn't use
`FatalErrorService`.

Looking for some feedback on how to properly solve this.

(cherry picked from commit 3a8bd70)
@kibanamachine kibanamachine merged commit 46a85bf into elastic:8.x Oct 30, 2024
25 checks passed
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #4 / APM API tests service_groups/service_group_count/service_group_count.spec.ts basic no archive Service group counts with alerts returns the correct number of alerts

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
core 450.8KB 452.2KB +1.4KB

cc @kowalczyk-krzysztof

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants